-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignore logging credentials #79
Ignore logging credentials #79
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add secret namespace also.
@@ -358,7 +358,7 @@ func processDeletedSecrets(ctx context.Context, rc client.Client, req types.Name | |||
} | |||
} | |||
|
|||
logger.V(2).Info("List of secrets with requested name", "secret-name", req.Name, "secretlist", sameNamedDestinationSecrets, "#secrets", len(sameNamedDestinationSecrets)) | |||
logger.V(2).Info("List of secrets with requested name", "secret-name", req.Name, "#secrets", len(sameNamedDestinationSecrets)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.V(2).Info("List of secrets with requested name", "secret-name", req.Name, "#secrets", len(sameNamedDestinationSecrets)) | |
logger.V(2).Info("List of secrets with requested name", "secret-name", req.Name, "secret length", len(sameNamedDestinationSecrets)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
logger.Info("Updating the destination secret", | ||
"current-data", currentDest.Data, | ||
"expected-data", expectedDest.Data) | ||
"secret", currentDest.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put this in single line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
1e5d0a5
to
e58b856
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Can we log the secret namespace as well
return err | ||
} | ||
logger.V(2).Info("Listing all the Peers connected to the Source", "SourceSecret", sourceSecret, "#connected-peers", len(uniqueConnectedPeers)) | ||
logger.V(2).Info("Listing all the Peers connected to the Source", "SourceSecret", sourceSecret.Name, "#connected-peers", len(uniqueConnectedPeers)) | ||
|
||
// anyErr will have the last found error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 58 in the file. We are logging the peerref instead of the secret name.
Please change that
@vbnrh: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
3ae0d17
to
46e04db
Compare
Signed-off-by: Gowtham Shanmugasundaram <gshanmug@redhat.com>
46e04db
to
58d1dff
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GowthamShanmugam, umangachapagain, vbnrh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-4.10 |
@umangachapagain: once the present PR merges, I will cherry-pick it on top of release-4.10 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@umangachapagain: new pull request created: #80 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes: #76
Signed-off-by: Gowtham Shanmugasundaram gshanmug@redhat.com